-
Notifications
You must be signed in to change notification settings - Fork 280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce --x-cmake-debug and --x-cmake-configure-debug #1173
Conversation
* Deduplicate the logic of "no entries means no filter, one or more entries means port filter" * Add unit test
TEST_CASE ("CMake debugger flags", "[arguments]") | ||
{ | ||
std::vector<std::string> t = {"--x-cmake-debug", | ||
"\\\\.\\pipe\\tespipe;zlib;bar;baz", | ||
"--x-cmake-configure-debug", | ||
"\\\\.\\pipe\\configure-pipe"}; | ||
auto v = VcpkgCmdArguments::create_from_arg_sequence(t.data(), t.data() + t.size()); | ||
auto& cmake_debug = v.cmake_debug.value_or_exit(VCPKG_LINE_INFO); | ||
REQUIRE(cmake_debug.value == "\\\\.\\pipe\\tespipe"); | ||
REQUIRE(!cmake_debug.is_port_affected("7zip")); | ||
REQUIRE(cmake_debug.is_port_affected("zlib")); | ||
REQUIRE(cmake_debug.is_port_affected("bar")); | ||
REQUIRE(cmake_debug.is_port_affected("baz")); | ||
REQUIRE(!cmake_debug.is_port_affected("bazz")); | ||
|
||
auto& cmake_configure_debug = v.cmake_configure_debug.value_or_exit(VCPKG_LINE_INFO); | ||
REQUIRE(cmake_configure_debug.value == "\\\\.\\pipe\\configure-pipe"); | ||
REQUIRE(cmake_configure_debug.is_port_affected("7zip")); | ||
REQUIRE(cmake_configure_debug.is_port_affected("zlib")); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only tests PortApplicableSetting
and not the actual cmake invocation.
I am more concerned about cmake <something> -P <script>
being a valid cmake call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would require setting up, as you said, the whole separate processes business. I don't have good ideas on testing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then testing by direct usage? I mean it is an --x-
option any way.
@@ -290,6 +316,18 @@ namespace vcpkg | |||
StabilityTag::Experimental, | |||
args.asset_sources_template_arg, | |||
msg::format(msgAssetSourcesArg)); | |||
{ | |||
std::string raw_cmake_debug; | |||
if (args.parser.parse_option(CMAKE_DEBUGGING_ARG, StabilityTag::Experimental, raw_cmake_debug)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if parse_option
couldn't be taught to do this automatically if fulfills the concept of being StringConstructable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could but then parse_option would have to be a template... if it were used in more than 1 place I think that'd be worth it but at least yet I don't think so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this:
https://learn.microsoft.com/en-us/vcpkg/commands/common-options#asset-sources
https://learn.microsoft.com/en-us/vcpkg/commands/common-options#binarysource
getting parsed?
Aren't those options using a kind of comma separation?
I think this is ready to land as soon as we have signoff on the corresponding docs PR. |
@BillyONeal Docs PR microsoft/vcpkg-docs#143 (Hopefully it doesn't get ghosted like the other docs PRs. ) |
Thanks for your contribution! |
* Add docs for --x-cmake-debug and --x-cmake-configure-debug See microsoft/vcpkg-tool#1173 * Add .mds. * add additional info * Update vcpkg/commands/common-options.md Co-authored-by: Victor Romero <[email protected]> * Minor text changes, added a URL to classic mode docs --------- Co-authored-by: Alexander Neumann <[email protected]> Co-authored-by: Victor Romero <[email protected]> Co-authored-by: Augustin Popa <[email protected]>
* Document "Reference/Commands/vcpkg contact" (#129) * add docs for contact * Update vcpkg/commands/contact.md --------- Co-authored-by: Victor Romero <[email protected]> * Add docs for --x-cmake-debug and --x-cmake-configure-debug (#142) * Add docs for --x-cmake-debug and --x-cmake-configure-debug See microsoft/vcpkg-tool#1173 * Add .mds. * add additional info * Update vcpkg/commands/common-options.md Co-authored-by: Victor Romero <[email protected]> * Minor text changes, added a URL to classic mode docs --------- Co-authored-by: Alexander Neumann <[email protected]> Co-authored-by: Victor Romero <[email protected]> Co-authored-by: Augustin Popa <[email protected]> --------- Co-authored-by: Javier Matos Denizac <[email protected]> Co-authored-by: Billy Robert O'Neal III <[email protected]> Co-authored-by: Alexander Neumann <[email protected]> Co-authored-by: Augustin Popa <[email protected]>
@Neumann-A FYI we just published a blog post on using these flags with the CMake Tools VS Code extension. Thanks again for your contribution! |
Debug with e.g.
./vcpkg.exe install zlib --x-cmake-debug \\.\pipe\tespipe;zlib
for portfile debugging
./vcpkg.exe install zlib --x-cmake-configure-debug \\.\pipe\tespipe;zlib
for debugging cmake configure of the port
(In Git bash
\
needs to be escaped to\\
)or more general
./vcpkg.exe install zlib --x-cmake(-configure)-debug <valid_pipe_name>(;semicolon_seperated_list_of_ports_to_debug)
for debugging cmake configure of the port
Requires external cmake debugger to attach to the opened pipe to continue execution (which is why I did not write tests for it since it would at least require two seperate process started at the right time to test this.).
Docs PR microsoft/vcpkg-docs#143